Skip to content

feat: make dev.up wait until services are actually running - #229

Open
pwnage101 wants to merge 1 commit into
masterfrom
pwnage101/enhance-startup-status-reporting
Open

feat: make dev.up wait until services are actually running#229
pwnage101 wants to merge 1 commit into
masterfrom
pwnage101/enhance-startup-status-reporting

Conversation

@pwnage101

@pwnage101 pwnage101 commented Jul 30, 2026

Copy link
Copy Markdown
Member

All too often I'll run dev.up and get all green, but eventually discover that half the services did not actually start for one reason or another. The main issue isn't having to fix them, it's not knowing which ones failed and/or not knowing that any failed at all. Poor UX.

Changes in this commit

Now, dev.up will not only report when each container has started, but also it will wait for the services to become healthy and exit fatally if any service fails to become healthy (most commonly due to missing imports).

Other bug fixes:

  • course-discovery has been broken in devstack for several months because it was still pointing at the upstream repo. I fixed both the repo URL and image name to point to the edx fork.
  • make dev.check.enterprise-access actually checks the correct port now.

This commit also just does a lot to make everything a bit more normalized and DRY across the board:

  • Consolidated common configs into common.yml.
  • Removed several absolute paths when relative ones were sufficient (see: WORKDIR).
  • Removed dead code which attempted to source legacy _env files which have been empty for years.
  • Migrated everything away from deprecated depends_on syntax lacking an explicit condition.
  • Removed dead references to historic analytics containers (hadoop, vertica, etc.)
  • Simplified check.sh to remove redundant health check commands (it reads docker-reported health now).

Demo recording

(Note that I edited the clip to skip ahead 5 minutes towards the end.)

This demonstrates how the requested service (enterprise-access) visually progresses from "Waiting" to "Healthy", extending the run time of dev.up. Afterwards, I intentionally removed a dependency (pytz) to demonstrate what happens when full health cannot be reached: the command crashes and this is desired.

Screen.Recording.2026-07-31.at.11.26.00-cut-merged-1785523402462.mov

Copilot AI review requested due to automatic review settings July 30, 2026 06:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves make dev.up reliability by having Docker Compose wait for containers to become healthy (not just “started”), and by normalizing service definitions so healthchecks and devserver commands are defined consistently across services.

Changes:

  • Update dev.up targets to run docker compose up -d --wait so the command only returns success once services are healthy.
  • Introduce common.yml base services (backend-app, microfrontend) and refactor many services to extends them, standardizing devserver loops and healthchecks.
  • Convert several depends_on entries to condition-based mappings to align with Compose readiness semantics.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
Makefile Adds --wait to docker compose up in dev.up targets so startup waits for healthy containers.
docker-compose.yml Refactors many services to inherit common devserver/healthcheck behavior and updates depends_on conditions.
common.yml Adds reusable base service definitions for backend apps and microfrontends, including healthcheck defaults.
microfrontend.yml Removes the old microfrontend base configuration in favor of common.yml.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docker-compose.yml Outdated
Comment thread docker-compose.yml Outdated
Comment thread docker-compose.yml
Comment thread docker-compose.yml
Comment thread common.yml Outdated
Copilot AI review requested due to automatic review settings July 30, 2026 07:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

common.yml:38

  • Typo in comment: dev.up.<servce> should be dev.up.<service>.
    # Configure a healthcheck to allow `make dev.up.<servce>` to give accurate feedback about

Makefile:272

  • docker compose up --wait requires a recent Docker Compose; older installs will error with an unknown flag. Since there’s no compose version/feature check here, the Make target will fail with a confusing error on those systems. Consider adding a quick feature check with a clear upgrade message before calling up --wait.
	docker compose up -d --wait $$(echo $* | tr + " ")

common.yml:47

  • healthcheck.start_interval is not supported by all Docker Engine / Docker Compose versions; if it's unsupported, docker compose will fail to parse this file and block all devstack startup. Consider removing it (or documenting/enforcing a minimum Docker version) and using a smaller interval instead.
      interval: 10s
      timeout: 10s
      retries: 20
      start_period: 240s
      start_interval: 5s  # poll fast during grace period so a healthy boot flips status quickly

Makefile:257

  • docker compose up --wait requires a recent Docker Compose; older installs will error with an unknown flag. Since there’s no compose version/feature check here, the Make target will fail with a confusing error on those systems. Consider adding a quick feature check with a clear upgrade message before calling up --wait.

This issue also appears on line 272 of the same file.

	docker compose up -d --wait --no-deps $$(echo $* | tr + " ")

Copilot AI review requested due to automatic review settings July 30, 2026 17:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (5)

common.yml:38

  • Typo in the comment: servceservice.
    # Configure a healthcheck to allow `make dev.up.<servce>` to give accurate feedback about

common.yml:47

  • healthcheck.start_interval is not supported by older Docker Engine / Compose combinations (the repo docs currently claim Docker 19.03+ is sufficient). Using this key can make docker compose fail to parse the file or silently ignore the intent on older installs. Consider removing it (falling back to interval) or updating the documented minimum Docker/Compose version.
      start_period: 240s
      start_interval: 5s  # poll fast during grace period so a healthy boot flips status quickly

common.yml:30

  • DEVSERVER_PRECHECK runs via eval but its exit status is currently ignored, so a failing precheck (e.g., failing pip install/bundle install/source command) will still fall through into the devserver loop and can mask the real failure. The precheck should fail fast like the DEVSERVER_CHECK block does.

This issue also appears on line 46 of the same file.

          if [ -n "$$DEVSERVER_PRECHECK" ]; then eval "$$DEVSERVER_PRECHECK"; fi

provision-lms.sh:15

  • Only provision-lms.sh was updated to use docker compose up --wait, but many other provision-*.sh scripts still use plain docker compose up -d ... (e.g., provision-discovery.sh, provision-ecommerce.sh, provision-insights.sh). If the goal is to avoid "all green" when services aren’t actually healthy, those scripts will still have the old behavior unless they’re updated too (or this script should document why it’s the exception).
    docker compose up -d --wait $app

Makefile:272

  • docker compose up --wait is a relatively new Compose feature; on older Compose plugin versions this will fail with an "unknown flag: --wait" error. Since the docs only mention a minimum Docker version (and don’t pin Compose), consider adding a Makefile preflight check with a clearer error message and/or updating the documented minimum Docker Compose plugin version.
dev.up.%: dev.check-memory ## Bring up services and their dependencies.
	docker compose up -d --wait $$(echo $* | tr + " ")

Copilot AI review requested due to automatic review settings July 30, 2026 19:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (19)

common.yml:38

  • Typo in comment: servceservice.
    # Configure a healthcheck to allow `make dev.up.<servce>` to give accurate feedback about

Makefile:272

  • docker compose up --wait requires a Docker Compose version that supports this flag. Without an explicit check, failures can be confusing and look like generic Compose errors. Adding a small guard makes the UX clearer.
	docker compose up -d --wait $$(echo $* | tr + " ")

provision-forum.sh:5

  • forum depends on services like mongo/memcached/elasticsearch/opensearch (docker-compose.yml:458-466). Using --no-deps prevents them from being started during provisioning, so forum may not boot and this script can fail.
docker compose up -d --wait --no-deps forum

provision-registrar.sh:10

  • registrar depends on services like redis/memcached/lms (docker-compose.yml:617-629). Using --no-deps here means those won't be started by provisioning, causing registrar to fail to boot and subsequent exec steps to fail.
docker compose up -d --wait --no-deps $name

provision-xqueue.sh:6

  • --no-deps disables bringing up xqueue dependencies. While xqueue currently depends on mysql80, relying on callers to have already started deps makes this script less robust during make dev.provision and inconsistent with other provisioning scripts.
docker compose up -d --wait --no-deps xqueue

provision-insights.sh:10

  • insights depends on mysql80/memcached (and others). Using --no-deps prevents those from starting during provisioning, so the container may not boot and later docker compose exec commands can fail.
docker compose up -d --wait --no-deps insights

provision-analyticsapi.sh:10

  • analyticsapi depends on mysql80 and elasticsearch (docker-compose.yml:588-595). Using --no-deps here prevents those from being started during make dev.provision, making the service likely to fail to boot and causing subsequent exec steps to fail.
docker compose up -d --wait --no-deps ${name}

provision-enterprise-access.sh:8

  • --no-deps prevents starting dependencies for both enterprise-access and lms (mysql80, redis, memcached, etc.). In make dev.provision, provision.sh doesn't start most of these, so this can leave the containers unhealthy and break the following provisioning steps.
docker compose up -d --wait --no-deps $name lms

provision-credentials.sh:13

  • --no-deps prevents starting credentials dependencies (e.g., lms/memcached). provision.sh does not start these, so make dev.provision can fail when docker compose exec runs against a container that never successfully started.
docker compose up -d --wait --no-deps $name

provision-notes.sh:12

  • --no-deps prevents starting Notes dependencies (e.g., mysql80/elasticsearch). Since provision.sh only guarantees mysql80/mongo are up, this can cause Notes to fail to start during provisioning.
docker compose up -d --wait --no-deps $name

provision-enterprise-subsidy.sh:11

  • Starting lms and enterprise-subsidy with --no-deps prevents required dependencies (mysql80, memcached, redis, etc.) from being started during provisioning. This can cause the services to fail to boot and later exec/LMS shell steps to fail.
docker compose up -d --wait --no-deps lms
docker compose up -d --wait --no-deps ${name}

provision-license-manager.sh:12

  • Starting license-manager and lms with --no-deps prevents their required dependencies (mysql80, redis, memcached, etc.) from being started during provisioning, which can break subsequent exec and LMS shell steps.
docker compose up -d --wait --no-deps $name
docker compose up -d --wait --no-deps lms

provision-discovery.sh:8

  • Starting lms, cms, and ecommerce with --no-deps prevents their dependencies from being started. This script immediately provisions Discovery and then calls into those services; if they aren't already fully up, this can fail intermittently.
docker compose up -d --wait --no-deps lms
docker compose up -d --wait --no-deps cms
docker compose up -d --wait --no-deps ecommerce

provision-coursegraph.sh:16

  • Starting coursegraph and cms with --no-deps prevents CMS dependencies (mysql80, elasticsearch, etc.) from being started if they aren't already up, making this script brittle when run via make dev.provision or standalone.
docker compose up -d --wait --no-deps coursegraph cms

provision-enterprise-catalog.sh:4

  • enterprise-catalog depends on services like mysql80 and memcached (docker-compose.yml:835-842). Using --no-deps prevents those dependencies from starting during make dev.provision, so the service may not boot and the subsequent provisioning commands can fail.
docker compose up -d --wait --no-deps $name

provision-edx-exams.sh:11

  • Starting lms and edx-exams with --no-deps prevents required dependencies from being started during provisioning. This can leave either service unhealthy and cause the later migration/shell steps to fail.
docker compose up -d --wait --no-deps lms
docker compose up -d --wait --no-deps ${name}

provision-designer.sh:5

  • Using --no-deps here prevents starting required dependencies for designer and lms during provisioning (e.g., mysql80, memcached, etc.). Since this script later runs migrations and creates OAuth clients via LMS, provisioning can fail if dependencies aren't started.
docker compose up -d --wait --no-deps $name --build
docker compose up -d --wait --no-deps lms

common.yml:47

  • start_interval is not supported by docker-compose v1 and can break docker compose parsing on older environments. Since this repo still has documentation referring to docker-compose 1.x (scripts/README.txt:25-29), consider removing this key for broader compatibility.
      start_interval: 5s  # poll fast during grace period so a healthy boot flips status quickly

Makefile:257

  • docker compose up --wait requires a sufficiently recent Docker Compose. If a developer still has older tooling (or only docker-compose), this target will fail with an unhelpful error. Consider adding a quick feature check to emit a clear upgrade message.

This issue also appears on line 272 of the same file.

	docker compose up -d --wait --no-deps $$(echo $* | tr + " ")

Comment thread provision-lms.sh Outdated
Comment thread common.yml Outdated
Copilot AI review requested due to automatic review settings July 30, 2026 19:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

common.yml:40

  • Typo in the usage comment: dev.up.<servce> should be dev.up.<service>.
    # Configure a healthcheck to allow `make dev.up.<servce>` to give accurate feedback about
    # whether the service actually started successfully and is serving traffic.
    # Set HEALTHCHECK_TARGET to the full URL curl should hit, e.g. "http://localhost:18000/heartbeat"

check:116

  • The enterprise-access check block won’t run when users pass enterprise-access because should_check is called with enterprise_access (underscore), and the health URL is pointing at the ecommerce port (18130) instead of enterprise-access (18270). This makes the check give false negatives/positives.
if should_check enterprise_access; then
    echo "Checking enterprise-access health:"
    run_check enterprise_access_heartbeat enterprise-access \
        "curl --fail -L http://localhost:18130/health/"
fi

diff:5

  • This looks like an accidentally committed patch/diff artifact. Keeping a file named diff containing diff --git ... output will confuse future readers and tooling; it should be removed from the repo/PR.
diff --git a/Makefile b/Makefile
index e78636a..c99172b 100644
--- a/Makefile
+++ b/Makefile
@@ -292,6 +292,9 @@ dev.restart-container: ## Restart all service containers.

check:12

  • Comment typo: “Exists 0” should be “Exits 0”.
# Exists 0 if successful; non-zero otherwise.

Copilot AI review requested due to automatic review settings July 30, 2026 20:05
@pwnage101
pwnage101 marked this pull request as draft July 30, 2026 20:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (20)

common.yml:40

  • Typo in the comment: dev.up.<servce> should be dev.up.<service>.
    # Configure a healthcheck to allow `make dev.up.<servce>` to give accurate feedback about
    # whether the service actually started successfully and is serving traffic.
    # Set HEALTHCHECK_TARGET to the full URL curl should hit, e.g. "http://localhost:18000/heartbeat"

provision-xqueue.sh:6

  • --no-deps prevents Compose from starting xqueue’s dependencies (e.g., mysql80) during provisioning. Since provision.sh only brings up mysql80/mongo, this can cause --wait to block/fail when xqueue can’t start cleanly. For provisioning, it’s safer to keep --wait but drop --no-deps so dependencies come up automatically.
# Bring up XQueue, we don't need the consumer for provisioning
docker compose up -d --wait --no-deps xqueue

provision-registrar.sh:10

  • --no-deps prevents registrar’s dependencies (mysql80, redis, etc.) from being started during provisioning. provision.sh does not start these, so docker compose up --wait --no-deps can hang/fail. Drop --no-deps here so Compose will bring up dependencies as needed.
docker compose up -d --wait --no-deps $name

provision-notes.sh:12

  • --no-deps prevents the notes service’s dependencies (mysql80, elasticsearch, etc.) from being started during provisioning, but provision.sh doesn’t start them. With --wait, this can block/fail. Drop --no-deps here so dependencies are started automatically.
docker compose up -d --wait --no-deps $name

provision-lms.sh:16

  • --no-deps prevents LMS/CMS dependencies (memcached, elasticsearch, mongo, etc.) from being started during provisioning, but provision.sh only starts mysql80/mongo. With --wait, this can block/fail when runserver can’t come up healthy. Drop --no-deps so Compose starts required dependencies.
for app in "${apps[@]}"; do
    docker compose up -d --wait --no-deps $app
done

provision-license-manager.sh:12

  • --no-deps prevents license-manager (and lms) dependencies from being started during provisioning. Since provision.sh doesn’t bring up redis/memcached/etc., --wait --no-deps can hang/fail. Drop --no-deps so dependencies are started automatically.
docker compose up -d --wait --no-deps $name
docker compose up -d --wait --no-deps lms

provision-insights.sh:10

  • --no-deps prevents insights’ dependencies (mysql80, analyticsapi, memcached, etc.) from being started during provisioning, but provision.sh doesn’t start them. With --wait, this can block/fail. Drop --no-deps so Compose starts required dependencies.
docker compose up -d --wait --no-deps insights

provision-forum.sh:5

  • --no-deps prevents forum’s dependencies (mongo, memcached, search services, etc.) from being started during provisioning, but provision.sh doesn’t start them. With --wait, this can block/fail. Drop --no-deps so Compose brings up dependencies.
docker compose up -d --wait --no-deps forum

provision-enterprise-subsidy.sh:11

  • --no-deps prevents enterprise-subsidy (and lms) dependencies from being started during provisioning, but provision.sh doesn’t start most of them (memcached, redis, etc.). With --wait, this can block/fail. Drop --no-deps so dependencies are started automatically.
docker compose up -d --wait --no-deps lms
docker compose up -d --wait --no-deps ${name}

provision-enterprise-catalog.sh:4

  • --no-deps prevents enterprise-catalog’s dependencies (mysql80, memcached, redis, etc.) from being started during provisioning, but provision.sh doesn’t start them. With --wait, this can block/fail. Drop --no-deps here.
docker compose up -d --wait --no-deps $name

provision-enterprise-access.sh:8

  • --no-deps prevents enterprise-access and lms dependencies from being started during provisioning. provision.sh only brings up mysql80/mongo, so --wait --no-deps can hang/fail. Drop --no-deps so Compose starts required dependencies.
docker compose up -d --wait --no-deps $name lms

provision-edx-exams.sh:11

  • --no-deps prevents edx-exams (and lms) dependencies from being started during provisioning, but provision.sh doesn’t start most of them. With --wait, this can block/fail. Drop --no-deps so dependencies are started automatically.
docker compose up -d --wait --no-deps lms
docker compose up -d --wait --no-deps ${name}

provision-discovery.sh:8

  • These services are started with --no-deps, which prevents their dependencies from being brought up during provisioning. Since provision.sh does not start redis/memcached/elasticsearch/etc., --wait --no-deps can hang/fail when these apps can’t become healthy. Drop --no-deps here so dependencies start automatically.
docker compose up -d --wait --no-deps lms
docker compose up -d --wait --no-deps cms
docker compose up -d --wait --no-deps ecommerce

provision-designer.sh:5

  • --no-deps prevents designer (and lms) dependencies from being started during provisioning. provision.sh only starts mysql80/mongo, so --wait --no-deps can hang/fail. Drop --no-deps so Compose starts dependencies.
docker compose up -d --wait --no-deps $name --build
docker compose up -d --wait --no-deps lms

provision-credentials.sh:13

  • --no-deps prevents credentials’ dependencies (mysql80, memcached, lms, etc.) from being started during provisioning, but provision.sh doesn’t start most of them. With --wait, this can block/fail. Drop --no-deps here.
docker compose up -d --wait --no-deps $name

provision-coursegraph.sh:16

  • --no-deps prevents coursegraph/cms dependencies from being started during provisioning. provision.sh doesn’t start most CMS deps (memcached, mongo, search, etc.), so --wait --no-deps can hang/fail. Drop --no-deps so dependencies are started automatically.
docker compose up -d --wait --no-deps coursegraph cms

provision-analyticsapi.sh:10

  • --no-deps prevents analyticsapi’s dependencies (mysql80, elasticsearch, etc.) from being started during provisioning, but provision.sh doesn’t start them. With --wait, this can block/fail. Drop --no-deps here.
docker compose up -d --wait --no-deps ${name}

check:4

  • This new check script is currently unused: the Makefile still runs bash ./check.sh $* (Makefile:335). Having both check and check.sh with the same content is confusing; either (a) remove this file, or (b) complete the rename by updating the Makefile and deleting check.sh.
#!/usr/bin/env bash
# Run checks for the provided service(s).
# To specify multiple services, separate them with spaces or plus signs (+).
# To specify all services, just pass in "all".

check:115

  • The enterprise-access check is currently incorrect: should_check uses enterprise_access (underscore), so it won’t match the actual service name enterprise-access, and it probes port 18130 (ecommerce) instead of 18270.
if should_check enterprise_access; then
    echo "Checking enterprise-access health:"
    run_check enterprise_access_heartbeat enterprise-access \
        "curl --fail -L http://localhost:18130/health/"

diff:5

  • This file appears to be an accidentally committed patch/dump (it contains a nested diff --git ... rather than code used by devstack). It should be removed from the repository to avoid confusion and accidental use.
diff --git a/Makefile b/Makefile
index e78636a..c99172b 100644
--- a/Makefile
+++ b/Makefile
@@ -292,6 +292,9 @@ dev.restart-container: ## Restart all service containers.

@pwnage101
pwnage101 force-pushed the pwnage101/enhance-startup-status-reporting branch 3 times, most recently from aeb55ed to 1b944a9 Compare July 31, 2026 18:14
@pwnage101
pwnage101 marked this pull request as ready for review July 31, 2026 18:45
Copilot AI review requested due to automatic review settings July 31, 2026 18:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

common.yml:29

  • Typo in comment: dev.up.<servce> -> dev.up.<service>.
    # Configure a healthcheck to allow `make dev.up.<servce>` to give accurate feedback about

common.yml:88

  • Same compatibility concern here: healthcheck.start_interval may break users on older Docker/Compose versions unless minimum versions are documented.
      interval: 10s
      timeout: 10s
      retries: 6
      start_period: 540s
      start_interval: 5s  # poll fast during grace period so a healthy boot flips status quickly

common.yml:46

  • healthcheck.start_interval is not supported by older Docker Engine / Compose combinations. This repo doesn't document minimum Docker/Compose versions, so including start_interval may make docker compose config/up fail for some developers. Consider either documenting the minimum required versions or removing start_interval to maximize compatibility.

This issue also appears on line 83 of the same file.

      interval: 10s
      timeout: 10s
      retries: 6
      start_period: 240s
      start_interval: 5s  # poll fast during grace period so a healthy boot flips status quickly

Comment thread provision-lms.sh
Comment thread provision-coursegraph.sh
All too often I'll run dev.up and get all green, but eventually discover
that half the services did not actually start for one reason or another.
The main issue isn't having to fix them, it's not knowing which ones
failed and/or not knowing that any failed at all. Poor UX.

Changes in this commit:

Now, dev.up will not only report when each container has *started*, but
also it will wait for the services to become *healthy* and exit fatally
if any service fails to become healthy (most commonly due to missing
imports).

Other bug fixes:

- course-discovery has been broken in devstack for several months
  because it was still pointing at the upstream repo. I Fixed both the
  repo URL and image name to point to the edx fork.
- make dev.check.enterprise-access actually checks the correct port now.

This commit also just does a lot to make everything a bit more
normalized and DRY across the board:

- Consolidated common configs into common.yml.
- Removed several absolute paths when relative ones were sufficient.
- Removed dead code which attempted to source legacy <service>_env files which have been empty for years.
- Migrated everything away from deprecated depends_on syntax lacking an explicit condition.
- Removed dead references to historic analytics containers (hadoop, vertica, etc.)
- Simplified check.sh to remove redundant health check commands (it reads docker-reported health now).
Copilot AI review requested due to automatic review settings July 31, 2026 19:56
@pwnage101
pwnage101 force-pushed the pwnage101/enhance-startup-status-reporting branch from 1b944a9 to 4079ef3 Compare July 31, 2026 19:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (6)

Makefile:277

  • This target now uses docker compose up --wait, but the make help description still says it only brings services up. Updating the ## text will keep CLI help and generated docs consistent with the new wait-for-health behavior.
dev.up.%: dev.check-memory ## Bring up services and their dependencies.

common.yml:82

  • Same as the backend-app healthcheck: quote $$HEALTHCHECK_TARGET so the CMD-SHELL invocation can’t interpret spaces or shell metacharacters from the URL.
      test: ["CMD-SHELL", "curl -fsS $$HEALTHCHECK_TARGET || exit 1"]

docs/devstack_interface.rst:204

  • This help listing entry says dev.up.without-deps.% only brings the service up, but it now uses --wait as well. Update this line so the documented make help output matches current behavior.
      dev.up.without-deps.%        Bring up services by themselves.

Makefile:257

  • The help text for this target no longer matches behavior: it now uses docker compose up --wait, so it will block until the service becomes healthy (when a healthcheck is defined). Update the ## description so make help and docs stay accurate.

This issue also appears on line 277 of the same file.

dev.up.without-deps.%: dev.check-memory ## Bring up services by themselves.

common.yml:40

  • The healthcheck command expands $$HEALTHCHECK_TARGET unquoted. Quoting avoids word-splitting and prevents shell metacharacters in the URL from being interpreted by CMD-SHELL.

This issue also appears on line 82 of the same file.

      test: ["CMD-SHELL", "curl -fsS $$HEALTHCHECK_TARGET || exit 1"]

docs/devstack_interface.rst:196

  • This help listing entry says dev.up.% only brings services up, but the implementation now waits for services to become healthy (docker compose up --wait). The documented make help output should reflect that behavior.

This issue also appears on line 204 of the same file.

      dev.up.%                     Bring up services and their dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants